Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Ability for custom storyshots testFunctions to utilise "snapshot per story file" #1841

Merged
merged 3 commits into from
Sep 14, 2017
Merged

Ability for custom storyshots testFunctions to utilise "snapshot per story file" #1841

merged 3 commits into from
Sep 14, 2017

Conversation

digitalmaster
Copy link
Contributor

@digitalmaster digitalmaster commented Sep 14, 2017 •
edited
Loading

Issue:

Can't use multiSnapshotWithOptions with custom test function because getSnapshotFileName is not exported.

What I did

This is a small tweak to the new multiSnapshotWithOptions test function. It basically just exports the getSnapshotFileName function so it can be used by those of use who have implemented our own custom test function. I also did a minor refactor and moved the getSnapshotFileName function to the utils.js file. Do let me know if this doesn't make any sense.

How to test

Go to storybook\addons\storyshots
and run yarn example

Is this testable with jest or storyshots?
Storyshots

Does this need a new example in the kitchen sink apps?
It should not.

Does this need an update to the documentation?
[Updated Docs] Yes.. We could mention this export in the docs so other users could become aware of it.

If your answer is yes to any of these, please make sure to include it in your PR.

@digitalmaster digitalmaster mentioned this pull request Sep 14, 2017
Merged
@codecov
Copy link

codecov bot commented Sep 14, 2017 •
edited
Loading

Codecov Report

Merging #1841 into release/3.3 will increase coverage by 0.01%.
The diff coverage is 75%.

Impacted file tree graph

@@               Coverage Diff               @@
##           release/3.3    #1841      +/-   ##
===============================================
+ Coverage        22.31%   22.32%   +0.01%     
===============================================
  Files              324      324              
  Lines             6322     6319       -3     
  Branches           797      795       -2     
===============================================
  Hits              1411     1411              
+ Misses            4323     4316       -7     
- Partials           588      592       +4
Impacted Files Coverage Δ
addons/storyshots/src/test-bodies.js 19.14% <ø> (-3.08%) ⬇️
addons/storyshots/src/index.js 80.85% <ø> (ø) ⬆️
addons/storyshots/src/utils.js 87.5% <75%> (-12.5%) ⬇️
...codemod/src/transforms/update-organisation-name.js 40.62% <0%> (ø) ⬆️
...react-native/src/manager/components/PreviewHelp.js 0% <0%> (ø) ⬆️
app/react-native/src/bin/storybook-build.js 0% <0%> (ø) ⬆️
.../ui/src/modules/ui/components/layout/dimensions.js 15.62% <0%> (ø) ⬆️
lib/codemod/src/transforms/update-addon-info.js 51.16% <0%> (ø) ⬆️
addons/info/src/components/Node.js 39.43% <0%> (ø) ⬆️
addons/knobs/src/components/types/Color.js 8.1% <0%> (ø) ⬆️
... and 31 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7a34c63...5c1d8ff. Read the comment docs.

@ndelangen
Copy link
Member

Fantastic first PR @digitalmaster !

I'd like @igor-dv to have a look before merging 👍

@ndelangen ndelangen changed the title Export getSnapshotFileName function for use in custom test functions Ability for custom storyshots testFunctions to utilise "snapshot per story file" Sep 14, 2017
igor-dv
igor-dv approved these changes Sep 14, 2017
View reviewed changes
Copy link
Member

@igor-dv igor-dv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool. Can you please add an example to the readme of how to use it ?

@digitalmaster
Copy link
Contributor Author

@ndelangen Thanks a lot! Glad others also find this useful :)

@igor-dv Added an example.. Let me know if it's unclear or has any bugs. 😬

@igor-dv
Copy link
Member

igor-dv commented Sep 14, 2017

@digitalmaster, thanks, looks great !

@igor-dv
Copy link
Member

igor-dv commented Sep 14, 2017

@digitalmaster, wanna merge ?

@digitalmaster
Copy link
Contributor Author

digitalmaster commented Sep 14, 2017 •
edited
Loading

Sure.. on to this 3.3 release right? storybooks:release/3.3? Just wanna confirm... Also don't think i have push access.

@igor-dv
Copy link
Member

igor-dv commented Sep 14, 2017

yeah to the 3.3

@digitalmaster
Copy link
Contributor Author

digitalmaster commented Sep 14, 2017 •
edited
Loading

@igor-dv I can't merge this PR myself without having push access to this repo right? Or am I missing something?

@igor-dv
Copy link
Member

igor-dv commented Sep 14, 2017

Ah, so let me push it then 😃

@igor-dv igor-dv merged commit f199465 into storybookjs:release/3.3 Sep 14, 2017
@digitalmaster
Copy link
Contributor Author

Awesome thanks.

@digitalmaster
Copy link
Contributor Author

Do you guys have any idea when another release will be created?

@digitalmaster digitalmaster deleted the jose/exportGetSnapshotFilenameFunc branch October 4, 2017 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@igor-dv igor-dv igor-dv approved these changes

@ndelangen ndelangen ndelangen approved these changes

Assignees

@digitalmaster digitalmaster

@igor-dv igor-dv

Labels
addon: storyshots feature request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@digitalmaster @ndelangen @igor-dv